Skip to content

Improve pidTPCML.cxx#1025

Merged
jgrosseo merged 1 commit into
AliceO2Group:masterfrom
ktf:improve-pid-tpc-ml
Jul 26, 2022
Merged

Improve pidTPCML.cxx#1025
jgrosseo merged 1 commit into
AliceO2Group:masterfrom
ktf:improve-pid-tpc-ml

Conversation

@ktf

@ktf ktf commented Jul 20, 2022

Copy link
Copy Markdown
Member
  • Out of line code to reduce memory usage while compiling pidTPC tasks
  • Change to use an array for the intermediate tracks representation.
    It would probably be better to read directly the values from the
    arrow tables, but that will come in a different PR.

@ktf
Improve pidTPCML.cxx
acbbe9d 
* Out of line code to reduce memory usage while compiling pidTPC tasks
* Change to use an array for the intermediate tracks representation.
  It would probably be better to read directly the values from the
  arrow tables, but that will come in a different PR.
@ktf ktf requested review from a team, alibuild, iarsene and jgrosseo as code owners July 20, 2022 09:19
@ktf

ktf commented Jul 20, 2022

Copy link
Copy Markdown
Member Author

@njacazio @ChSonnabend could you have a look? Besides the mechanical change of moving the inference code in a separate .cxx file (this is probably one of the causes why pidTPC takes so much memory to compile), I have also changed the intermediate track representation to be an std::array, which should already somewhat speedup the whole thing. A better approach would be to find a way to have ONNX runtime digest a columnwise representation so that it can bulk process the whole arrow table, rather than one track at the time.

@ChSonnabend

ChSonnabend commented Jul 20, 2022

Copy link
Copy Markdown
Contributor

Hi Giulio,
Thanks a lot for the PR! I think this makes sense to put it in a separate .cxx file. It makes the code much cleaner.
Indeed I also mentioned the idea of running directly on the tables to Nicolo before, but I wasn't sure how to implement it. If you know how to do this, that would be very nice!

@ktf

ktf commented Jul 20, 2022

Copy link
Copy Markdown
Member Author

@iarsene could you review and eventually approve?

@alibuild

alibuild commented Jul 20, 2022 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Error while checking build/O2Physics/o2 for acbbe9d at 2022-07-26 06:59:

## sw/BUILD/O2Physics-latest/log
c++: fatal error: Killed signal terminated program cc1plus
ninja: build stopped: subcommand failed.

Full log here.

@njacazio

njacazio commented Jul 20, 2022

Copy link
Copy Markdown
Collaborator

Hi, indeed, thanks a lot this was needed!

@njacazio njacazio mentioned this pull request Jul 21, 2022
Merged
@jgrosseo jgrosseo merged commit d8d8d62 into AliceO2Group:master Jul 26, 2022
lbariogl pushed a commit to CF-tutorials/O2Physics that referenced this pull request Jul 28, 2022
@ktf @lbariogl
Improve pidTPCML.cxx (AliceO2Group#1025)
cf63cf3 
* Out of line code to reduce memory usage while compiling pidTPC tasks
* Change to use an array for the intermediate tracks representation.
  It would probably be better to read directly the values from the
  arrow tables, but that will come in a different PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jgrosseo jgrosseo jgrosseo approved these changes
@alibuild alibuild Awaiting requested review from alibuild alibuild is a code owner
@iarsene iarsene Awaiting requested review from iarsene iarsene is a code owner

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ktf @ChSonnabend @alibuild @njacazio @jgrosseo